Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix geodata files permissions #20

Closed
wants to merge 1 commit into from
Closed

fix geodata files permissions #20

wants to merge 1 commit into from

Conversation

amitrea
Copy link
Contributor

@amitrea amitrea commented Nov 30, 2023

Fixes immich-app/immich#5404

Please check the issue above for more details.

How to test:

  1. Build dev and prod images from scratch:
$ docker build --no-cache --target dev -t docker.svc.local/private/base-server-dev:20231130 .
$ docker build --target prod -t docker.svc.local/private/base-server-prod:20231130 .
  1. Run the prod base server image:
$ docker run -it -d --name immich-base-server docker.svc.local/private/base-server-prod:20231130
  1. Log into immich-base-server:
$ docker exec -it immich-base-server bash
  1. Check permissions for files in /usr/src/resources. It should show rw- for owner, r-- for group, r-- for others.
$ ls -lah /usr/src/resources
total 34M
drwxr-xr-x. 1 root root  128 Nov 30 11:08 .
drwxr-xr-x. 1 root root   18 Nov 30 11:08 ..
-rw-r--r--. 1 root root 136K Nov 30 02:58 admin1CodesASCII.txt
-rw-r--r--. 1 root root 2.2M Nov 30 02:58 admin2Codes.txt
-rw-r--r--. 1 root root  32M Nov 30 03:51 cities500.txt
-rw-r--r--. 1 root root   25 Nov 30 11:07 geodata-date.txt

@bo0tzz
Copy link
Member

bo0tzz commented Nov 30, 2023

Fwiw, it's also possible to use a --chmod flag on the ADD directive (https://docs.docker.com/engine/reference/builder/#add)

@bo0tzz bo0tzz requested a review from zackpollard November 30, 2023 12:31
@amitrea
Copy link
Contributor Author

amitrea commented Nov 30, 2023 •
edited
Loading

Initially I wanted to do that for all ADD commands, but why multiple layers for each ADD and RUN instead of 1 layer. All files related to geodata are somehow related. Isn't it?

Not to mention it is more verbose with adding --chmod=644 for each ADD command.
Impact on the dev image is a bit better.

@zackpollard
Copy link
Contributor

zackpollard commented Dec 1, 2023 •
edited
Loading

Fixed in #21 using --chmod on the ADD commands. As this happens in the dev/builder layer, we don't mind multiple layers and the resulting dockerfile is a bit cleaner. Thanks for the PR 😄

@zackpollard zackpollard closed this Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrasm91 jrasm91 jrasm91 approved these changes

@zackpollard zackpollard Awaiting requested review from zackpollard

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Error importing geodata when running container with a non-root user
4 participants
@amitrea @bo0tzz @zackpollard @jrasm91